Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DNM: IBC-Gaia Integration #130

Closed
wants to merge 31 commits into from
Closed

DNM: IBC-Gaia Integration #130

wants to merge 31 commits into from

Conversation

mossid
Copy link

@mossid mossid commented Sep 17, 2019

This PR is the tracking PR for Gaia/IBC integration. To try it out, pull down this branch and build gaia using the instructions in cmd/gaiacli/README.md.

@mossid mossid force-pushed the joon/ibc-gaia-interface branch from bec3cdb to 38e34ca Compare September 17, 2019 16:45
@codecov-io
Copy link

codecov-io commented Sep 17, 2019

Codecov Report

Merging #130 into master will increase coverage by 0.81%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #130      +/-   ##
==========================================
+ Coverage   65.65%   66.46%   +0.81%     
==========================================
  Files           5        5              
  Lines         492      498       +6     
==========================================
+ Hits          323      331       +8     
+ Misses        138      136       -2     
  Partials       31       31

@jackzampolin
Copy link
Member

jackzampolin commented Sep 30, 2019

I'm in the process of turning this into a 2 chain tutorial. I'll be adding notes here:

  • CLI commands aren't piped through the correct output function and don't respect --output text or --indent
  • Lack of input validation on CLI
  • Lack of error returns for improperly formatted store returns

@jackzampolin jackzampolin marked this pull request as ready for review October 1, 2019 15:26
@wlsaud619
Copy link

https://github.com/cosmos/gaia/tree/joon/ibc-gaia-interface/cmd/gaiacli

I tried to test ibc by referring to the above document.
However, gaiacli does not seem to have any ibc related commands.

  • gaiacli version
$ gaiacli --home ibc0/n0/gaiacli version --long

name: gaia
server_name: gaiad
client_name: gaiacli
version: 0.0.0-68-g5e81764
commit: 5e817641bb5256b80dab6928f45a65ba6b907786
build_tags: netgo,ledger
go: go version go1.13 linux/amd64

  • gaiacli query
$ gaiacli --home ibc0/n0/gaiacli q ibc client path
Querying subcommands

Usage:
  gaiacli query [command]

Aliases:
  query, q

Available Commands:
  account                  Query account balance

  tendermint-validator-set Get the full tendermint validator set at given height
  block                    Get verified data for a the block at given height
  txs                      Query for paginated transactions that match a set of tags
  tx                       Query for a transaction by hash in a committed block

  auth                     Querying commands for the auth module
  mint                     Querying commands for the minting module
  distribution             Querying commands for the distribution module
  slashing                 Querying commands for the slashing module
  supply                   Querying commands for the supply module
  staking                  Querying commands for the staking module
  gov                      Querying commands for the governance module

Flags:
  -h, --help   help for query

Global Flags:
      --chain-id string   Chain ID of tendermint node
  -e, --encoding string   Binary encoding (hex|b64|btc) (default "hex")
      --home string       directory for config and data (default "/data/node/.gaiacli")
  -o, --output string     Output format (text|json) (default "text")
      --trace             print out full stack trace on errors

Use "gaiacli query [command] --help" for more information about a command.

  • gaiacli tx
$ gaiacli --home ibc0/n0/gaiacli tx ibc
Transactions subcommands

Usage:
  gaiacli tx [command]

Available Commands:
  send         Create and sign a send tx

  sign         Sign transactions generated offline
  multisign    Generate multisig signatures for transactions generated offline

  broadcast    Broadcast transactions generated offline
  encode       Encode transactions generated offline
  decode       Decode an amino-encoded transaction string

  staking      Staking transaction subcommands
  slashing     Slashing transactions subcommands
  distribution Distribution transactions subcommands
  gov          Governance transactions subcommands
  crisis       Crisis transactions subcommands

Flags:
  -h, --help   help for tx

Global Flags:
      --chain-id string   Chain ID of tendermint node
  -e, --encoding string   Binary encoding (hex|b64|btc) (default "hex")
      --home string       directory for config and data (default "/data/node/.gaiacli")
  -o, --output string     Output format (text|json) (default "text")
      --trace             print out full stack trace on errors

Use "gaiacli tx [command] --help" for more information about a command.

@@ -87,8 +87,8 @@ else
endif

install: go.sum check-ledger
go install -mod=readonly $(BUILD_FLAGS) ./cmd/gaiad
go install -mod=readonly $(BUILD_FLAGS) ./cmd/gaiacli
go install $(BUILD_FLAGS) ./cmd/gaiad
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmmm how comes? Did you have any issues with -mod=readonly? Is there anything I can help out with?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! I was having issues with -mod=readonly!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this the case @alessio?

@jackzampolin
Copy link
Member

@wlsaud619 It looks like that isn't the right version of gaia.

@wlsaud619
Copy link

@jackzampolin Sorry, my mistake. I didn't apply branch ;)

gaiacli --home ibc0/n0/gaiacli query ibc channel channel ibc-mock chan0 --trust-node
```

## Send Packet
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both send-packet commands are using ibc0, but all the other sections have matched pairs of ibc0/ibc1 commands. Is this intentional? I'm guessing it's just a typo.

Copy link
Member

@jackzampolin jackzampolin Oct 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should the commands look? This part of the instructions are left over from the loopback demo

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flush command is currently using ibc0 as its home but making transaction on ibc1, which is bit misleading

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mossid so how would you rewrite the example?

@fedekunze fedekunze added the WIP label Oct 3, 2019
@fedekunze fedekunze changed the title IBC-Gaia Integration DNM: IBC-Gaia Integration Oct 3, 2019
cmd/gaiacli/README.md Outdated Show resolved Hide resolved
cmd/gaiacli/README.md Outdated Show resolved Hide resolved
@wlsaud619
Copy link

Packet send/received well.
However, it seems that the information is not updated at the part of querying channel information.

image

@jackzampolin
Copy link
Member

@wlsaud619 It looks like the sequence is updating for me after the flush.

gaiacli --home ibc0/n0/gaiacli q ibcmocksend sequence chan0

# Returns the next expected sequence number, for use in scripting
gaiacli --home ibc0/n0/gaiacli q ibcmocksend next chan0
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a fresh install:

Running gaiacli --home ibc0/n0/gaiacli q ibcmocksend sequence chan0 returns 0

Running gaiacli --home ibc0/n0/gaiacli q ibcmocksend next chan0 immediately afterward returns 2 (without having sent any packets)

I was able to reproduce the above after stopping the testnet gaiad processes, removing the entire ibc-testnets directory, and starting over. Will try on a new machine tomorrow (currently, on a Mac).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wanted to add that manually inputting the correct sequence number (should be 1 instead of 2) to successfully send a packet the first time, then using the above to get subsequent "next sequence" numbers works.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/cosmos/cosmos-sdk/blob/e69029151525422924e384cd9175778fa8533c51/x/ibc/mock/send/client/cli/query.go#L70

In L74 of the function add 1 to res, so if value is nil, res should be set to 0 instead of 1.

--node1 tcp://localhost:26657 \
--node2 tcp://localhost:26557 \
--chain-id2 ibc1 \
--from1 n0 --from2 n1 -y -o text
Copy link

@kphed kphed Oct 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The -y option doesn't seem to exist for this build and running this command results in this error:
ERROR: unknown shorthand flag: 'y' in -y

Ran gaiacli --home ibc0/n0/gaiacli tx ibc channel flush ibcmocksend --help and didn't see the option/flag listed either (edit: oh lol it was to confirm the transaction - getting late here).

@fedekunze fedekunze mentioned this pull request Oct 29, 2019
5 tasks
@stale
Copy link

stale bot commented Dec 12, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants